Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Activate draft Icon for active report #2823

Merged
merged 1 commit into from
May 14, 2021
Merged

Activate draft Icon for active report #2823

merged 1 commit into from
May 14, 2021

Conversation

parasharrajat
Copy link
Member

@parasharrajat parasharrajat commented May 12, 2021 •
edited
Loading

Please review

Details

Fixed Issues

Fixes #2807

Tests / QA Steps

  1. Select a chat
  2. Begin writing a comment
  3. Look at the LHN and there won't should be a pencil icon
  4. Switch to another chat
  5. Look at the LHN and you'll see the pencil icon for the previous chat you were in

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

WEB

draft.mp4

DESKTOP

draft-d.mp4

Android

draft-and.mp4

Mobile-WEB

draft-ios-web.mp4

@parasharrajat parasharrajat requested a review from a team as a code owner May 12, 2021 16:56
@MelvinBot MelvinBot requested review from NikkiWines and removed request for a team May 12, 2021 16:56
NikkiWines
NikkiWines reviewed May 12, 2021
View reviewed changes
Copy link
Contributor

@NikkiWines NikkiWines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, could we please get screenshots/videos across all platforms? Thanks

cc: @Julesssss since you approved the proposal

@Julesssss
Copy link
Contributor

Still missing screenshots, please add them next time.

Julesssss
Julesssss requested changes May 13, 2021
View reviewed changes
Copy link
Contributor

@Julesssss Julesssss left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It tested well for me on each platform except mobile Web (iOS 12).

Notice that the icon will not display when leaving a chat, it is only when you tap into a chat again that the icon updates.

Screen.Recording.2021-05-13.at.10.55.33.mov

@parasharrajat
Copy link
Member Author

@Julesssss Let me check that part. I sure that is the default implementation.

@parasharrajat
Copy link
Member Author

@Julesssss I was going to upload all videos after some time.

It tested well for me on each platform except mobile Web (iOS 12).

I am not facing any issues. I tested it on both IOS-web and android-web but it works fine (Video linked in description). Also, most importantly it can either work on all or none.

Julesssss
Julesssss approved these changes May 14, 2021
View reviewed changes
Copy link
Contributor

@Julesssss Julesssss left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested again and this time around it worked as expected, not sure what happened yesterday 🤷‍♂️

Also, most importantly it can either work on all or none.

Yeah, it seemed like a separate issue if anything -- but it was worth raising anyway.

@parasharrajat
Copy link
Member Author

@NikkiWines All yours and screens updated. Thanks.

NikkiWines
NikkiWines approved these changes May 14, 2021
View reviewed changes
Copy link
Contributor

@NikkiWines NikkiWines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

@NikkiWines NikkiWines merged commit 45b99bb into Expensify:main May 14, 2021
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify OSBotify mentioned this pull request May 14, 2021
Closed
26 tasks
@OSBotify
Copy link
Contributor

🚀 Deployed to staging in version: 1.0.46-1🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@parasharrajat parasharrajat deleted the draft branch November 20, 2023 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Julesssss Julesssss Julesssss approved these changes

@NikkiWines NikkiWines NikkiWines approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Have the draft icon always be visible in the LHN when there is a draft
4 participants
@parasharrajat @Julesssss @OSBotify @NikkiWines